-
Notifications
You must be signed in to change notification settings - Fork 790
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix WithSpan annotation instrumentation #929
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
} | ||
|
||
// OpenTelemetry Auto Annotations shaded so that it can be used in opentelemetry-api-beta | ||
// instrumentation, and then its usage can be unshaded after OpenTelemetry API is shaded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to confirm this means our unit tests can catch the issue in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately, no, this was needed for the fix. currently at least we have to rely on smoke tests to catch shading-related problems. probably we should create a whole smoke test around instrumentation OpenTelemetry API, since that instrumentation is shading-fragile. i will open issue........ (a couple min later) oh ya, i have an issue already for that: #298 😄
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
||
@RestController | ||
public class WebController { | ||
@RequestMapping("/greeting") | ||
public String greeting() { | ||
return "Sup Dawg"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good change
I will continue spreading confusion :) First, I don't understand what exactly was broken? Our instrumentation depended and referenced |
It stopped working as soon as The reason it stopped working is because the type of
was shaded to call
and then |
Resolves #913